CBOR bindings #559
CBOR bindings #559
Conversation
awscrt/cbor.py
Outdated
| return _awscrt.cbor_encoder_get_encoded_data(self._binding) | ||
|
|
||
| def write_int(self, val: int): | ||
| # TODO: maybe not support bignum for now. Not needed? |
There was a problem hiding this comment.
smithy currently does not support anything above 64bits afaik so we can probably skip bignum for now
awscrt/cbor.py
Outdated
| # For negative value, the value to encode is -1 - val. | ||
| val_to_encode = -1 - val | ||
| bit_len = val_to_encode.bit_length() | ||
| if bit_len > 64: |
There was a problem hiding this comment.
wouldnt python need 65 bits to represent - u64_max? so this logic might be a bit off, since we dont need bignum for - u64_max
awscrt/cbor.py
Outdated
| Args: | ||
| val (int): value to be encoded and written to the encoded data. | ||
| """ | ||
| assert isinstance(val, int) |
There was a problem hiding this comment.
for my education, havent we already indicated in function signature htat val is an int? why do we have to check twice
There was a problem hiding this comment.
The function signature is just a type hint, it won't enforce anything for runtime
https://docs.python.org/3/library/typing.html
Note
The Python runtime does not enforce function and variable type annotations. They can be used by third party tools such as type checkers, IDEs, linters, etc.
awscrt/cbor.py
Outdated
|
|
||
| def write_array_start(self, number_entries: int): | ||
| """Add a start of array element. | ||
| A legistic with the `number_entries` |
| Args: | ||
| number_entries (int): number of entries in the array to be written | ||
| """ | ||
| if number_entries < 0 or number_entries > 2**64: |
There was a problem hiding this comment.
ugh, cbor spec allows 2^64 entries? not sure we would be able to allocate cbor chunk for that even if all entries are 1 byte. should we consider a lower bound
There was a problem hiding this comment.
The implementation will not allocate the whole array, it appends to the buffer when user keeps writing to it. So, the size of the array won't cause any allocation problem.
DmitriyMusatkin
dismissed
their stale review
accidental approve. comments still need to be discussed
…on into cbor-remove-py-impl
…on into cbor-remove-py-impl
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.